Skip to content

Conversation

@benderliz
Copy link
Contributor

@benderliz benderliz commented Dec 18, 2024

@robodoo
Copy link
Collaborator

robodoo commented Dec 18, 2024

Pull request status dashboard

@benderliz benderliz changed the base branch from 18.0 to 17.0 December 18, 2024 20:54
@C3POdoo C3POdoo requested a review from a team December 18, 2024 20:56
@benderliz benderliz self-assigned this Dec 18, 2024
@benderliz benderliz added the 2 label Dec 18, 2024
@benderliz benderliz force-pushed the 17.0-inventory-fix-reorder-rules branch from f1c8c9e to edf767d Compare December 18, 2024 21:22
Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @benderliz
Thank you for addressing the task feedback so quickly!
Approving with a few comments below; please address what you agree with and can merge in when you're done 👍

Also, for this:

I will need to revise this whole doc with new screenshots for 18.0

Feel free to make the similar, targeted navigational revision in 18.0 as a new quick PR like this one (another 1-2 point), and then for that larger revision (or complete rewrite), do as a follow-up PR to that for another 3 or 5 points.

Although related, these are 3 separate tasks (2 address the community feedback, and 1 is your own initiative) so if we similarly execute them in this modular way it becomes a) easier for us to track changes and b) you get the full point value 🤙

..
@robodoo delegate=benderliz

Comment on lines 69 to 71
- :guilabel:`On Hand`: The number of units currently available in inventory.
- :guilabel:`Forecast`: The number of units expected to be available in inventory after all orders
are taken into account.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you added more context to the UI here. I think for this particular operation, the On Hand and Forecast values are not editable for a New rule line (on my screen it's grayed out and the cursor goes right to the Route field, screenshot below)...

Screenshot 2024-12-18 at 4 54 46 PM

...which, makes sense since Route Min Quantity Max Quantity and To Order are the inputs directly tied to the RR, whereas the other two are more closely tied to making an inventory adjustment.

My suggestion is to remove these list items, or optionally add a see also admonition that points to any relevant docs on how to make an inventory adjustment.

@StraubCreative
Copy link
Contributor

@fw-bot ignore up to saas-17.4

@benderliz
Copy link
Contributor Author

Hi @benderliz Thank you for addressing the task feedback so quickly! Approving with a few comments below; please address what you agree with and can merge in when you're done 👍

Also, for this:

I will need to revise this whole doc with new screenshots for 18.0

Feel free to make the similar, targeted navigational revision in 18.0 as a new quick PR like this one (another 1-2 point), and then for that larger revision (or complete rewrite), do as a follow-up PR to that for another 3 or 5 points.

Although related, these are 3 separate tasks (2 address the community feedback, and 1 is your own initiative) so if we similarly execute them in this modular way it becomes a) easier for us to track changes and b) you get the full point value 🤙

.. @robodoo delegate=benderliz

Hi @StraubCreative! Thanks so much for the quick review, and I appreciate all the direction. That makes a lot of sense - after making revisions/merging this one, I will open a PR for the navigational change in 18.0, and then I'll open a new task for making longer fixes in a separate PR. Cheers!

@benderliz benderliz removed the request for review from jero-odoo December 19, 2024 17:10
@benderliz benderliz force-pushed the 17.0-inventory-fix-reorder-rules branch from edf767d to c7f77ef Compare December 19, 2024 17:54
@benderliz benderliz removed the request for review from a team December 19, 2024 17:55
@benderliz
Copy link
Contributor Author

@robodoo r+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants